-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Relay upgrade #373
Relay upgrade #373
Conversation
60a23bd
to
b208715
Compare
72cbd31
to
b7df04e
Compare
c8047e8
to
0c7d521
Compare
bc8257f
to
bd41443
Compare
bd41443
to
e7d78ff
Compare
69234ee
to
a585b4d
Compare
2d6472a
to
656633a
Compare
8a4dab5
to
98f9bed
Compare
@@ -71,6 +72,32 @@ export async function publishTransaction( | |||
}); | |||
} | |||
|
|||
switch (chainId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here you can use the data-driven programming approach as @PetarKirov suggested
But it can be done in a following PR.
export enum NetworkConfig { | ||
Pratter = 'pratter', | ||
Mainnet = 'mainnet', | ||
Sepolia = 'sepolia', | ||
Chiado = 'chiado', | ||
} | ||
|
||
export function isSupportedFollowNetwork(network: string): boolean { | ||
return Object.values(NetworkConfig).includes(network as NetworkConfig); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remind me why Enum but not union type?
Not a blocker, just wandering.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enum is what I had in mind, remade it with union as it makes more sence
vendor/snarkjs
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks scary. It's nice to see it worked out!
.env.example
Outdated
BEACON_REST_API_PRATTER= | ||
BEACON_REST_API_MAINNET= | ||
BEACON_REST_API_SEPOLIA= | ||
BEACON_REST_API_CHIADO= | ||
REDIS_HOST=localhost |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are so many empty? Smt forgotten or?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The public api-s I know of are not very stable so I wouldnt recommend them(could still add those here I guess), the Light-client contracts(LC_Sepolia) and the Hashi contracts(Sepolia_Hashi) are for whoever is using DendrETH to create them and add to his .env file and as for the RPCs yes I can add those but anyone can find them with a quick google search so I dont feel like thats necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huge PR but with a good commit history! Thanks!
Left a couple of comments, none of which a blocker.
150bb83
98f9bed
to
150bb83
Compare
Implement changes that were proposed by reviews of #284
Those suggestions can be found in comments in the above PR and on: https://coda.io/d/_d6vM0kjfQP6#My-Tasks_tuy0e/r1012&view=modal